-
Notifications
You must be signed in to change notification settings - Fork 67
Add a structure that holds information about vulkan devices (Vulkan Profile JSON) #938
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
@Erfan-Ahmadi note that we can't use GPUinfo.org cause its on AGPL license, so its either |
… write information about available extensions
| renderdoc_api_t* const m_rdoc_api; | ||
| const VkPhysicalDevice m_vkPhysicalDevice; | ||
|
|
||
| core::string& m_profile; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what are you doing!?
you do realize the vector grows and reallocates ? you need an index into it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I forgot that it might behave like that. Let me correct that
src/nbl/video/CVulkanConnection.cpp
Outdated
| } | ||
| } | ||
|
|
||
| NBL_API2 int vulkaninfo(const std::span<const std::string_view> args) { return ::vulkaninfo(args); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove NBL_API2, you are in cpp file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's a copy-paste remnant, I'm going to remove it in the next commit
smoke/main.cpp
Outdated
| return false; | ||
| } | ||
|
|
||
| nbl::video::vulkaninfo(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you defined it with const std::span<const std::string_view> but calling without args.. start testing your code before you push
src/nbl/video/CVulkanConnection.cpp
Outdated
| return true; | ||
| } | ||
|
|
||
| void CVulkanConnection::exportGpuProfiles() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this function and move it's body straight to Smoke - dump profiles on disk with known outputs (-o, check --help) and log, CI should print them
src/nbl/video/CVulkanConnection.cpp
Outdated
| for (size_t i = 0;; i++) | ||
| { | ||
| auto arg = "--json=" + std::to_string(i); | ||
| int code = ::vulkaninfo(std::array<const std::string_view, 1>{ arg }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw you could just to_array instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't even know that function exists XD
thanks
| core::smart_refctd_ptr<system::ILogger>&& logger, const SFeatures& featuresToEnable | ||
| ); | ||
|
|
||
| static void exportGpuProfiles(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/nbl/video/CVulkanConnection.cpp
Outdated
| // TODO: move inside `create` and call it LOG_FAIL and return nullptr | ||
| #define LOG(logger, ...) if (logger) {logger->log(__VA_ARGS__);} | ||
|
|
||
| extern int vulkaninfo(const std::span<const std::string_view>); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you didn't have to change original argc & argv signature, you just had to call it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alright, it's going to make things simpler in vulkaninfo. I'll use span in nabla only
Description
Testing
TODO list: